Summary

Correct fix for a real and broader-than-Prebid defect: IntegrationSettings::normalize() ran from finalize_deserialized(), so it applied to Settings::from_toml (adapters) and Settings::from_json_value — the runtime config-store blob path in config_payload.rs:39. Every string in every integration's config was re-parsed as JSON, turning publisherId = "12345" into 12345 before it reached the auction request. Scoping the coercion to #[cfg(test)] and calling it only from the already-#[cfg(test)] from_toml_and_env helper is the right layering, since deserialize_with = "from_value_or_str" is the sanctioned per-field string-tolerance mechanism.

I verified the removal is safe rather than assuming it: EdgeZero v0.0.4's apply_env_overlay (edgezero-core/src/app_config.rs:404-480) coerces each env string into the existing TOML value's type and explicitly rejects array/table targets. So no untyped strings reach production config, and the env-var doc examples removed from prebid.rs documented a capability production never had.

Requesting changes on two points: a missing operator-facing migration note, and four leftover tests that now assert behavior no production path has.

Blocking

🔧 wrench

• Operator-visible behavior change has no CHANGELOG entry (CHANGELOG.md): This is not only a Prebid fix — it changes type handling for all 15 integration configs. None of their enabled: bool fields use deserialize_with = "from_value_or_str" (checked every file in crates/trusted-server-core/src/integrations/), so a config carrying enabled = "true" or timeout_ms = "1000" previously loaded fine and now fails at startup with invalid type: string "true", expected a boolean. Relatedly, is_explicitly_disabled can no longer see enabled = "false", so that config becomes a parse error instead of a clean disable — loud failure is arguably the better outcome, but it should be an intentional, documented one. Unreleased → Changed already documents an equivalent bid_param_zone_overrides breaking change; please add a sibling entry telling operators to audit quoted scalars in [integrations.*].

❓ question

• Four legacy-env tests still assert behavior production does not have (settings.rs:2032 — details inline).

Non-blocking

♻️ refactor

• Regression test stops one layer short of the defect (settings.rs:3200 — details inline).

🤔 thinking

• PR description understates the scope: the title and body read as Prebid-only, but the behavior change spans every integration's config and every non-test load path. Worth stating so reviewers and operators reading the merge commit understand the blast radius.

📌 out of scope

• Docs still advertise integration env-var overrides, including forms EdgeZero rejects: docs/guide/integrations-overview.md:293-308, docs/guide/error-reference.md:117-123, and trusted-server.example.toml:163 (TRUSTED_SERVER__CREATIVE_OPPORTUNITIES__SLOT='[{…}]' — an array override the overlay refuses). These were already inaccurate before this PR, but removing the equivalent Prebid examples here makes the inconsistency conspicuous. Worth a follow-up issue.

⛏ nitpick

• normalize_legacy_env lost its doc comment and the val → value rename is diff noise (settings.rs:213 — details inline).

👍 praise

• Fixed at the source instead of special-casing Prebid — deleting the blanket coercion rather than adding a string-preserving exception for bid_param_overrides removes a whole class of silent retyping.
• The new test is a genuine regression test (settings.rs:3200) — cherry-picked onto main it fails with left: Number(12345), right: String("12345"), and it covers bid_param_zone_overrides and bid_param_override_rules in addition to the two shapes named in the PR body.

CI Status

All 19 GitHub checks pass (fmt, clippy, cargo test across fastly/axum/cloudflare/spin/CLI, cross-adapter parity, integration tests, vitest, CodeQL).

Independently reproduced locally against a worktree of the head commit:

• cargo fmt --all -- --check: PASS
• cargo clippy-axum: PASS
• cargo test -p trusted-server-core: PASS (1647 passed, 0 failed; main baseline 1648)
• cargo test-axum: PASS

Summary

Deletes IntegrationSettings::normalize / normalize_env_value and its call in finalize_deserialized, so integration config values are no longer recursively re-parsed as JSON. This is the right fix for #932: the recursive re-parse ran on every load path (from_toml and from_json_value, via finalize_deserialized), so a TOML publisherId = "12345" was silently rewritten to the number 12345 before it ever reached PrebidIntegrationConfig.

Verified the removal is safe for production paths:

• Settings::from_toml_and_env — the only source that ever produced JSON-encoded strings for integration values — is already #[cfg(test)]-gated and has no callers outside settings.rs tests.
• The live path is ts config push → BlobEnvelope → settings_from_config_blob → from_json_value. EdgeZero's app-config env overlay (edgezero-core coerce_env_value, v0.0.4) coerces each env value to the existing TOML leaf's type and errors otherwise, so no untyped strings reach IntegrationSettings from the overlay.
• Quoted scalars now surface loudly rather than silently: validate_settings_for_deploy → validate_enabled_integrations parses every integration at ts config push time, so enabled = "true" fails there instead of coercing.

Local verification (native host target): cargo test -p trusted-server-core --lib integrations::prebid → 143 passed; --lib settings → 118 passed; --lib config → 99 passed.

Approving — no blocking findings.

Non-blocking

📌 out of scope

• Env-override docs still advertise the removed behavior: docs/guide/configuration.md:142 (BIDDERS='["kargo","rubicon"]') and docs/guide/configuration.md:1071-1073 (BID_PARAM_OVERRIDES, BID_PARAM_ZONE_OVERRIDES, BID_PARAM_OVERRIDE_RULES as JSON strings) document exactly the JSON-string decoding this PR deletes. The PR removes the identical examples from the rustdoc on PrebidIntegrationConfig, so the two now disagree. Acknowledged and tracked in #972 — noting it only so the follow-up isn't lost, since an operator copying those lines today gets a config that no longer applies.

♻️ refactor

• No test locks in the new failure behavior: the CHANGELOG entry claims "quoted numeric and boolean scalars now fail validation instead of silently converting", but nothing asserts it. The deleted test_env_var_roundtrip_normalizes_integration_types was the only test that pinned the old direction, and it went away without a replacement pinning the new one. A small test in settings.rs — TOML with [integrations.testlight] enabled = "true", asserting integration_config::<TestlightConfig> (or validate_settings_for_deploy) returns a configuration error — would keep a future refactor from quietly reintroducing coercion.

📝 note

• Upgrade ordering deserves a CHANGELOG line: the breaking-change entry tells operators to audit [integrations.*], but the already-pushed app_config blob was produced by the old CLI and still has the coerced values baked in (publisherId stored as a number). Deploying the new WASM without re-running ts config push leaves #932 unfixed with no error anywhere. Worth one sentence: re-push config after upgrading.

🌱 seedling

• from_toml_and_env is now a test-only fixture with no production analogue (crates/trusted-server-core/src/settings.rs:1995): with normalize gone it exercises config-crate Environment semantics that no runtime path uses, while the real overlay lives in edgezero-core. The remaining tests built on it (test_handlers_override_with_env, the ec/partner env tests) are now testing a legacy helper rather than shipped behavior. Not for this PR, but the helper looks ready to retire.

👍 praise

• Regression test targets the real path (crates/trusted-server-core/src/integrations/prebid.rs:5722): asserting through serde_json::to_value → from_json_value mirrors the actual BlobEnvelope transition rather than only from_toml, and it covers all three override surfaces (static, zone, canonical rule) in one pass. That is the version of this test that would have caught #932.

CI Status

• fmt: PASS
• clippy: PASS (covered by the per-adapter check/build jobs)
• rust tests: PASS (cargo test, axum native, cloudflare native + wasm32-unknown-unknown, spin native + wasm32-wasip1, ts CLI native, cross-adapter parity)
• js tests: PASS (vitest, format-typescript, format-docs)
• prepare integration artifacts: still pending at review time — worth a glance before merge.

Summary

Re-review of the reworked PR. All findings from the prior CHANGES_REQUESTED review are resolved:

• The global JSON re-interpretation (normalize / normalize_env_value) is deleted, not shimmed; no production path called it.
• The regression test now runs the full production shape — TOML → Settings → runtime JSON → typed PrebidIntegrationConfig → compiled-rule OpenRTB emission — and asserts numeric-looking strings survive static, zone, and canonical-rule merges. Ported onto main it fails with Number(12345) vs String("12345"), so it is a genuine regression test.
• CHANGELOG documents the breaking config change.
• Deleting the two datadome env tests is not a coverage loss — protection_scope.rs retains 8 dedicated tests.

Approving. Two non-blocking notes inline.

CI Status

Locally reproduced against a worktree of the head commit:

• cargo fmt --all -- --check: PASS
• cargo clippy-axum: PASS
• cargo test -p trusted-server-core: PASS (1729 passed, 0 failed)

GitHub checks green except two still-running at review time (cargo test (axum native), prepare integration artifacts).